feat(otelcollector): render OTel Collector by tianfeng92 · Pull Request #5106 · tigera/operator

tianfeng92 · 2026-07-24T16:13:31Z

Summary

Render the OTel Collector as a StatefulSet in calico-system, configured via LogCollector.spec.otelCollector.

The collector receives logs from fluent-bit via OTLP and optionally federates Prometheus metrics, forwarding both to user-configured OTLP endpoints. It is added to the LogCollector CR rather than AdditionalStores because it is operator-managed infrastructure (StatefulSet, ConfigMap, RBAC, certs) with its own lifecycle, not a pointer to an external system.

Release Note

Add OpenTelemetry Collector component to export logs and metrics via OTLP.

Test plan

Unit tests for render, controller, and fluent-bit pipeline OTel output
Validated on MCM cluster — all three log types exporting, clean collector startup

EV-6862

hjiawei · 2026-07-24T18:31:34Z

+	DefaultMemoryLimit         = "512Mi"
+	DefaultMemoryRequest       = "128Mi"
+	DefaultMemoryLimitMiB      = 409  // 80% of 512Mi
+	DefaultMemorySpikeLimitMiB = 100  // ~25% of limit_mib


How are these memory limits decided? Are they from the vendor recommendations?

The memory_limiter processor follows the OTel Collector best practices: limit_mib should be ~80% of the container memory limit (409 ≈ 80% of 512Mi), and spike_limit_mib at ~25% of limit_mib (100 ≈ 25% of 409). This headroom lets the GC reclaim memory before the container hits OOMKill. See: https://github.com/open-telemetry/opentelemetry-collector/blob/main/processor/memorylimiterprocessor/README.md

The 512Mi container limit itself is a conservative starting point — the collector is mostly I/O bound (receiving and forwarding telemetry), so memory usage is dominated by in-flight batches rather than computation.

pasanw · 2026-08-04T22:33:41Z

+	}
+
+	if logCollector.Spec.OTelCollector == nil {
+		r.status.OnCRNotFound()


We shouldn't mark the CR as not found in this case. We should enforce it at the CRD level (via kubebuilder annotations) that the OTelCollector field must be set. We can have a defensive check here if you want, but it should SetDegraded with a message rather that OnCRNotFound

Why OnCRNotFound() is right here - it doesn't just skip the reconcile, it deletes the TigeraStatus object. So openTelemetry set → a log-collector-otel status exists; unset → no object at all. That's the behaviour you want for an opt-in feature.

operator/pkg/controller/status/status.go

Lines 319 to 324 in a8c2d9e

if m.enabled != nil && !*m.enabled {

// This status manager is explicitly disabled, because the controller has called OnCRNotFound.

// Remove any TigeraStatus object that had previously been created, and skip updating the status.

m.removeTigeraStatus()

return

}

Why SetDegraded would hurt - LogCollector exists on every Enterprise cluster for fluent-bit, and openTelemetry is an optional field on it. Degrading when that field is unset would put a permanently Degraded log-collector-otel in kubectl get tigerastatus on every cluster that hasn't enabled OTel.

SetDegraded no-ops until OnCRFound() has been called, and on that path it hasn't been.

operator/pkg/controller/status/status.go

Lines 1134 to 1137 in a8c2d9e

if m.enabled == nil || !*m.enabled {

// Never set any conditions unless the status manager is enabled.

return

}

On the CRD marker - making openTelemetry required applies to every LogCollector, so existing fluent-bit-only CRs would fail validation on next apply.

Your underlying concern was valid though and worse than flagged. An empty or half-filled spec rendered a config otelcol rejects at boot, so the pod crash-looped while status sat on Progressing with no reason. Now fixed: present-but-invalid degrades with a specific message, plus MinItems=1 on exporters.

So: absent → OnCRNotFound. Present but invalid → SetDegraded. Work for you?

pasanw · 2026-08-04T22:34:14Z

+		return reconcile.Result{}, nil
+	}
+
+	r.status.OnCRFound()


Move this beneath the block where we find the CR

Other controllers can call this right after the lookup because their CR only exists when the feature is on. Ours is different: every cluster has a LogCollector for fluent-bit, so "CR exists" doesn't mean "OpenTelemetry is enabled".

If I move it up, a cluster not using OTel calls OnCRFound() and then OnCRNotFound() three lines later — creating and deleting the log-collector-otel TigeraStatus on every reconcile.

Leaving it below both checks means the status object exists exactly when export is configured. Happy to move it if you still prefer.

pasanw · 2026-08-05T00:02:31Z

+
+	objs = append(objs, secret.ToRuntimeObjects(secret.CopyToNamespace(OTelCollectorNamespace, c.cfg.PullSecrets...)...)...)
+
+	return objs, nil


What is our deletion strategy to ensure that if otel is disabled, we don't keep the otel collector and all of its resources around?

I added the recycle. The component now returns its resources in toDelete when OTel is off, same as fluent-bit does for its DaemonSet on license expiry. Triggers are: feature switched off, license loses the feature, and license expired.

Verified on a cluster — all 7 resources go, and come back on re-enable.

pasanw · 2026-08-05T01:46:18Z

+    endpoint: {{.Endpoint}}
+{{- if .TLSInsecure}}
+    tls:
+      insecure: true


Claude: This renders the wrong OTel setting. The tlsInsecure field is documented as "disables TLS verification" (keep TLS on, just don't check the cert), but tls.insecure: true means no TLS at all — plaintext. The setting for "skip verification" is insecure_skip_verify, which we never render.

So for a self-signed target like the field doc suggests:

http:// or a scheme-less host:port → plaintext; telemetry (incl. flow data) goes out unencrypted.

https:// grpc → the scheme forces TLS and overrides insecure, so the self-signed cert fails verification and the connection breaks; tlsInsecure did nothing.

Either way the field can't do what it promises. If plaintext is actually what we want, that's a separate thing (or just an http:// scheme).

Done in

operator/pkg/render/otelcollector/collector-config.yaml.template

Lines 48 to 70 in fca9047

exporters:

{{- range .Exporters}}

{{.Prefix}}/{{.Name}}:

endpoint: {{.Endpoint}}

{{- if .Plaintext}}

# http:// endpoint: no transport security. "insecure" here means no TLS at

# all, which is what the scheme asks for.

tls:

insecure: true

{{- else}}

tls:

# Load the system roots alongside any user CA, so publicly-signed and

# privately-signed endpoints both verify. Verification is never disabled.

include_system_ca_certs_pool: true

{{- if $.ExporterCAFile}}

ca_file: {{$.ExporterCAFile}}

{{- end}}

{{- if .MutualTLS}}

cert_file: {{$.ExporterClientCertFile}}

key_file: {{$.ExporterClientKeyFile}}

{{- end}}

{{- end}}

{{- end}}

…lCollector Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Renames the API surface from OTel to OpenTelemetry, and reworks exporter TLS, certificate rotation, validation and network policy. API: - OTel* types become OpenTelemetry*; LogCollector.spec.otelCollector becomes spec.openTelemetry. The file moves to api/v1/opentelemetry_types.go. - Replace tlsInsecure with mutualTLS. tlsInsecure rendered tls.insecure, which disables TLS entirely rather than skipping verification, so it could never do what it documented. A private CA is now supplied out of band via the otel-collector-ca ConfigMap and a client keypair via the otel-collector-client-certs Secret, following the syslog/Splunk and external Elasticsearch conventions. Plaintext is selected by an http:// endpoint, as with Splunk. No input disables verification. - Add listType/listMapKey, required and default markers on exporters. Certificate rotation: - Roll the pod on certificate change via HashAnnotations. The config hash alone never changes on rotation because the config holds paths, not PEM, so a rotated CA would silently break fluent-bit ingest until something else restarted the collector. - Watch tigera-ca-private, and reload the receiver's CA and keypair in place so rotation usually costs no restart. Validation: - Reject specs the collector cannot start from (no exporters, no data sources, duplicate exporter names) instead of rendering a config it rejects at boot and leaving status on Progressing with no reason. - Degrade when the exporter CA or client keypair exists but is empty. - Degrade rather than return silently when the Installation is unreadable. Network policy: - Pin egress to each exporter's destination and drop the blanket rule that allowed any host on the OTLP ports. In-cluster destinations match by Service, since Calico resolves Domains rules from observed DNS answers and those never match a ClusterIP reached via the cluster domain. - Set Source on both ingress rules. The internal metrics port serves without TLS or authentication and was reachable by any pod. Lifecycle: - Remove the collector's resources when the feature is disabled, the license loses the feature, or the license expires. Nothing owns them, so they previously lingered, and the expiry message claimed forwarding had stopped while the collector kept exporting. - Pin to a single replica. Each replica federates the same Prometheus targets under its own service.instance.id, duplicating every series. Also register the receiver keypair's key usages, give the probes a boot grace, derive the memory limiter from the container's effective memory limit so overrides take effect, and use the current otlp_grpc/otlp_http exporter type names. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Adds a ServiceMonitor for the collector so a silently failing export pipeline is visible: queue depth, export failures and dropped records are otherwise only in the pod's logs. Gated on the collector actually being configured, and removed when it is not. The ServiceMonitor selects a Service that only exists while the collector does, so rendering it unconditionally leaves one behind matching nothing once OpenTelemetry export is switched off. The monitor controller reads LogCollector to decide, mirroring how LicenseExpired already gates the other ServiceMonitors. The collector's identity constants move to pkg/render so both packages can share them: pkg/render/otelcollector imports pkg/render/monitor for the Prometheus federation target, so monitor cannot import it back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…e Collector The field name was changed to OpenTelemetry but its comment still opened with "configures the OpenTelemetry Collector", which is the framing the review pushed back on: it implies the struct mirrors upstream Collector configuration, which it does not. Describe it as configuring OpenTelemetry export, which the operator happens to implement with a Collector. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

tianfeng92 requested review from a team and marvin-tigera as code owners July 24, 2026 16:13

marvin-tigera added this to the v1.44.0 milestone Jul 24, 2026

marvin-tigera added docs-pr-required release-note-required labels Jul 24, 2026

tianfeng92 added docs-pr-required release-note-required labels Jul 24, 2026

hjiawei reviewed Jul 24, 2026

View reviewed changes

Comment thread internal/controller/otelcollector_controller.go Outdated

hjiawei reviewed Jul 24, 2026

View reviewed changes

Comment thread pkg/render/otelcollector/component.go Outdated

tianfeng92 force-pushed the EV-6862-otel-collector branch 7 times, most recently from 8124957 to 222f3f4 Compare July 27, 2026 23:24

tianfeng92 changed the title ~~feat(otelcollector): render OTel Collector from LogCollector.spec.otelCollector~~ feat(otelcollector): render OTel Collector Jul 28, 2026

tianfeng92 force-pushed the EV-6862-otel-collector branch 2 times, most recently from 6dd36c8 to a8c2d9e Compare July 28, 2026 17:55

pasanw requested changes Aug 5, 2026

View reviewed changes

tianfeng92 and others added 4 commits August 6, 2026 14:42

feat(otelcollector): render OTel Collector from LogCollector.spec.ote…

65a64bc

…lCollector Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

tianfeng92 force-pushed the EV-6862-otel-collector branch from c612900 to fca9047 Compare August 6, 2026 22:12

tianfeng92 requested a review from pasanw August 6, 2026 23:02

	if m.enabled != nil && !*m.enabled {
	// This status manager is explicitly disabled, because the controller has called OnCRNotFound.
	// Remove any TigeraStatus object that had previously been created, and skip updating the status.
	m.removeTigeraStatus()
	return
	}

	if m.enabled == nil \|\| !*m.enabled {
	// Never set any conditions unless the status manager is enabled.
	return
	}


		objs = append(objs, secret.ToRuntimeObjects(secret.CopyToNamespace(OTelCollectorNamespace, c.cfg.PullSecrets...)...)...)

		return objs, nil

	exporters:
	{{- range .Exporters}}
	{{.Prefix}}/{{.Name}}:
	endpoint: {{.Endpoint}}
	{{- if .Plaintext}}
	# http:// endpoint: no transport security. "insecure" here means no TLS at
	# all, which is what the scheme asks for.
	tls:
	insecure: true
	{{- else}}
	tls:
	# Load the system roots alongside any user CA, so publicly-signed and
	# privately-signed endpoints both verify. Verification is never disabled.
	include_system_ca_certs_pool: true
	{{- if $.ExporterCAFile}}
	ca_file: {{$.ExporterCAFile}}
	{{- end}}
	{{- if .MutualTLS}}
	cert_file: {{$.ExporterClientCertFile}}
	key_file: {{$.ExporterClientKeyFile}}
	{{- end}}
	{{- end}}
	{{- end}}

Uh oh!

Conversation

tianfeng92 commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Release Note

Test plan

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tianfeng92 commented Jul 24, 2026 •

edited

Loading